Append only git checkouts

diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs
index eeffcb05b724d7e5a7044729c3f6848c4d52aa3f..b0f1053ef1dc234d735e273fa05999bb375c4bae 100644 (file)
--- a/src/cargo/sources/git/source.rs
+++ b/src/cargo/sources/git/source.rs
@@ -123,21 +123,13 @@ impl<'cfg> Registry for GitSource<'cfg> {
 
 impl<'cfg> Source for GitSource<'cfg> {
     fn update(&mut self) -> CargoResult<()> {
-        let lock = try!(self.config.lock_git());
+        let lock = try!(self.config.git_path()
+            .open_rw(".cargo-lock-git", self.config, "the git checkouts"));
 
         let db_path = lock.parent().join("db").join(&self.ident);
 
-        let reference_path = match self.source_id.git_reference() {
-            Some(&GitReference::Branch(ref s)) |
-            Some(&GitReference::Tag(ref s)) |
-            Some(&GitReference::Rev(ref s)) => s,
-            None => panic!("not a git source"),
-        };
-        let checkout_path = lock.parent().join("checkouts")
-            .join(&self.ident).join(reference_path);
-
         // Resolve our reference to an actual revision, and check if the
-        // databaes already has that revision. If it does, we just load a
+        // database already has that revision. If it does, we just load a
         // database pinned at that revision, and if we don't we issue an update
         // to try to find the revision.
         let actual_rev = self.remote.rev_for(&db_path, &self.reference);
@@ -157,9 +149,14 @@ impl<'cfg> Source for GitSource<'cfg> {
             (try!(self.remote.db_at(&db_path)), actual_rev.unwrap())
         };
 
+        let checkout_path = lock.parent().join("checkouts")
+            .join(&self.ident).join(actual_rev.to_string());
+
         // Copy the database to the checkout location. After this we could drop
         // the lock on the database as we no longer needed it, but we leave it
         // in scope so the destructors here won't tamper with too much.
+        // Checkout is immutable, so we don't need to protect it with a lock once
+        // it is created.
         try!(repo.copy_to(actual_rev.clone(), &checkout_path, &self.config));
 
         let source_id = self.source_id.with_precise(Some(actual_rev.to_string()));
@@ -167,9 +164,6 @@ impl<'cfg> Source for GitSource<'cfg> {
                                                     &source_id,
                                                     self.config);
 
-        // Cache the information we just learned, and crucially also cache the
-        // lock on the checkout location. We wouldn't want someone else to come
-        // swipe our checkout location to another revision while we're using it!
         self.path_source = Some(path_source);
         self.rev = Some(actual_rev);
         self.path_source.as_mut().unwrap().update()

diff --git a/src/cargo/util/config.rs b/src/cargo/util/config.rs
index 2f207d86eabc8bfac9c8993a2000a9c77eae91d2..e9988fe94966b126e0fe0160859db89db8163526 100644 (file)
--- a/src/cargo/util/config.rs
+++ b/src/cargo/util/config.rs
@@ -16,7 +16,7 @@ use toml;
 use core::shell::{Verbosity, ColorConfig};
 use core::MultiShell;
 use util::{CargoResult, CargoError, ChainError, Rustc, internal, human};
-use util::{Filesystem, FileLock, LazyCell};
+use util::{Filesystem, LazyCell};
 
 use util::toml as cargo_toml;
 
@@ -32,7 +32,6 @@ pub struct Config {
     extra_verbose: Cell<bool>,
     frozen: Cell<bool>,
     locked: Cell<bool>,
-    git_lock: LazyCell<FileLock>,
 }
 
 impl Config {
@@ -49,7 +48,6 @@ impl Config {
             extra_verbose: Cell::new(false),
             frozen: Cell::new(false),
             locked: Cell::new(false),
-            git_lock: LazyCell::new(),
         }
     }
 
@@ -71,21 +69,6 @@ impl Config {
         self.home_path.join("git")
     }
 
-    /// All git sources are protected by a single file lock, which is stored
-    /// in the `Config` struct. Ideally, we should use a lock per repository,
-    /// but this leads to deadlocks when several instances of Cargo acquire
-    /// locks in different order (see #2987).
-    ///
-    /// Holding the lock only during checkout is not enough. For example, two
-    /// instances of Cargo may checkout different commits from the same branch
-    /// to the same directory. So the lock is acquired when the first git source
-    /// is updated and released when the `Config` struct is destroyed.
-    pub fn lock_git(&self) -> CargoResult<&FileLock> {
-        self.git_lock.get_or_try_init(|| {
-            self.git_path().open_rw(".cargo-lock-git", self, "the git checkouts")
-        })
-    }
-
     pub fn registry_index_path(&self) -> Filesystem {
         self.home_path.join("registry").join("index")
     }